Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

Addresses #10

cc @kriskowal @prashantv

@prashantv Thanks a lot for the feedback! Within Hailstorm, and also generally speaking, it seems like the Limiter interface is meant to be used explicitly in the case when some operation is meant to be repeated and maintain a specific RPS - i.e. there would probably need to be blocking in the most common use cases.

In that case, having the goroutine block as a result of the call to the Limiter seems like the most complete experience, from a users' perspective, and frees me from having to rebuild blocking code which might be error prone.

For testing, I'm certainly open to adding back the mock clock since I like the abstraction as well - lemme see if there's a way of accomplishing that.

Also @prashantv , correct me if I'm wrong but the old interface still blocking, is it not? The docs as well as the implementation state that the Limiter is meant to sleep till the rps is met.

Is there any interest in reviving this PR? I have a use case where I would like to be able to pass the Limiter a Context so that it can return early if the Context is cancelled. Currently, the code looks like the following:

type Thing struct {
    limiter ratelimit.Limiter
}

func (t *Thing) Do(ctx context.Context) error {
    t.limiter.Take()

    select {
    case <-ctx.Done():
         return ctx.Err()
    default:
    }

    // expensive work ...
}

The code works as is, but it would be nice, and more efficient, if I could combine the two steps into one:

func (t *Thing) Do(ctx context.Context) error {
    if _, err := <-t.limiter.TakeWithCtx(ctx); err != nil {
        return err
     }

    // expensive work ...
}

Implementation-wise it should, in theory, be relatively trivial to support such a method. The heart of TakeWithCtx could look like the following:

func (t *limiter) TakeWithCtx() (time.Time, error) {
    // setup work to determine the amount of time we should sleep for ...

    timer := time.NewTimer(t.sleepFor)
    select {
    case <-ctx.Done():
        // possible cleanup work ...

        timer.Stop()
        return time.Time{}, ctx.Error()
    case <-timer.C:
    }

    // ...
}

In practice I imagine this change might be more difficult though because:

1. It requires a new method, TakeWithCtx, on the Limiter interface which will be a breaking change.
2. It requires the Clock interface to be re-done as TakeWithCtx won't work if the Sleep method is blocking, another breaking change.

I'd be curious to know if anyone else see values in such a change?

This repo has no releases so breaking changes are fine. I don't think there's really an owner right now -- this is a copy of the rate limiting code in yab which I wrote for limiting the number of requests sent by yab. However, this library isn't used by yab, we can iterate on the APIs to find something that makes sense for everyone.

I'm up for reviving this PR, if it will prove useful. I'll spend some time time addressing @prashantv's comments from last time and update.

Thanks @achals, that would be great! I looked into this a little yesterday and figured I would share some thoughts I had in case it would be helpful.

The documentation for the Clock interface states:

Clock is the minimum necessary interface to instantiate a rate limiter with a clock or mock clock, compatible with clocks created using github.com/andres-erbsen/clock.

I took a look at the github.com/andres-erbsen/clock package and the interface for Clock is:

type Clock interface {
    Now() time.Time
    Sleep(d time.Duration)
    Timer(d time.Duration) *Timer
    // other methods ...
}

So we could add a Timer(d time.Duration) *Timer method to the Clock interface in the package and still be compatible with the Clock interface in the github.com/andres-erbsen/clock package (though we would have to reuse the Timer struct from ithub.com/andres-erbsen/clock).

TakeWithCtx could then be exactly the same as Take except we could replace the call to t.clock.Sleep(t.sleepFor) with:

var (
    err error
    timer = t.clock.Timer(t.sleepFor)
)

select {
case <-ctx.Done():
    err = ctx.Error()
case <-timer.C:

if err != nil {
    // ...
}